-
Notifications
You must be signed in to change notification settings - Fork 0
chore: 통계 시간 단위를 ms로 변경 #65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough통계 기능의 저장소 계층을 리팩터링하여 StudySessionRepository의 복잡한 통계 쿼리를 새로운 StatisticsRepository로 이동하고, 시간 단위 메서드명을 초(seconds)에서 밀리초(milliseconds)로 일관되게 변경했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java`:
- Around line 53-57: 메서드 getTwoHourSlotStats의 선언에 불필요하게 포함된 파라미터
`@Param`("dayEnd") LocalDateTime dayEnd을 제거하세요: TwoHourSlotStatistics
getTwoHourSlotStats(`@Param`("dayStart") LocalDateTime dayStart, `@Param`("userId")
Long userId); 또한 이 시그니처 변경에 맞춰 해당 메서드를 호출하는 모든 위치(및 관련 테스트)에서 dayEnd 인자를 제거하거나,
만약 실제로 dayEnd를 쿼리에서 사용하려는 의도라면 현재 쿼리 내에서 dayEnd를 참조하도록 수정해 일관성을 유지하세요.
🧹 Nitpick comments (3)
src/main/java/com/gpt/geumpumtabackend/statistics/dto/TwoHourSlotStatistics.java (1)
6-6: 네이밍 일관성 검토 권장.다른 DTO들은
*Millis접미사를 사용하는 반면(getTotalStudyMillis,getAverageDailyMillis등), 이 메서드는getMillisecondsStudied로 다른 패턴을 사용합니다. 일관성을 위해getStudiedMillis또는getSlotMillis로 변경하는 것을 고려해 주세요.♻️ 일관된 네이밍 제안
- Integer getMillisecondsStudied(); + Integer getStudiedMillis();src/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java (2)
106-120: 불필요한COALESCE사용.Line 109의
COALESCE(s.end_time, d.day_end)는 실제로 실행되지 않는 코드입니다. Line 120의 JOIN 조건s.end_time > d.day_start에서 이미end_time이 NULL인 세션이 필터링되므로, SELECT 절에서s.end_time은 항상 non-NULL입니다. 코드 가독성을 위해COALESCE를 제거하는 것이 좋습니다.Based on learnings, 완료된 세션만 통계에 포함되어야 하며, 현재 JOIN 조건으로 이 요구사항이 충족됩니다.
♻️ 제안: 불필요한 COALESCE 제거
GREATEST( 0, TIMESTAMPDIFF( MICROSECOND, GREATEST(s.start_time, d.day_start), - LEAST(COALESCE(s.end_time, d.day_end), d.day_end) + LEAST(s.end_time, d.day_end) ) / 1000 )
194-207: 주간 통계와 동일한 불필요한COALESCE패턴.Line 197의
COALESCE(s.end_time, d.day_end)도 주간 통계와 동일하게 불필요한 코드입니다. Line 207의 JOIN 조건에서 NULLend_time이 이미 제외됩니다.♻️ 제안: 불필요한 COALESCE 제거
GREATEST( 0, TIMESTAMPDIFF( MICROSECOND, GREATEST(s.start_time, d.day_start), - LEAST(COALESCE(s.end_time, d.day_end), d.day_end) + LEAST(s.end_time, d.day_end) ) / 1000 )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/gpt/geumpumtabackend/statistics/dto/DayMaxFocusAndFullTimeStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/dto/MonthlyStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/dto/TwoHourSlotStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/dto/WeeklyStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.javasrc/main/java/com/gpt/geumpumtabackend/statistics/service/StatisticsService.javasrc/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java
💤 Files with no reviewable changes (1)
- src/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-11T11:48:29.664Z
Learnt from: kon28289
Repo: Geumpumta/backend PR: 16
File: src/main/java/com/gpt/geumpumtabackend/study/repository/StudySessionRepository.java:264-287
Timestamp: 2025-11-11T11:48:29.664Z
Learning: In the Geumpumta backend project, study session statistics (daily, weekly, monthly, grass) intentionally exclude in-progress sessions where end_time is NULL. Only completed sessions (with non-null end_time) should be counted in the statistics calculations.
Applied to files:
src/main/java/com/gpt/geumpumtabackend/statistics/dto/WeeklyStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/dto/DayMaxFocusAndFullTimeStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/service/StatisticsService.javasrc/main/java/com/gpt/geumpumtabackend/statistics/dto/MonthlyStatistics.javasrc/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test (Gradle + Testcontainers)
🔇 Additional comments (8)
src/main/java/com/gpt/geumpumtabackend/statistics/dto/WeeklyStatistics.java (1)
4-6: LGTM!타입 선택이 적절합니다.
Long은 주간 총 밀리초를 안전하게 담을 수 있고,Integer는 일일 평균(최대 ~86,400,000ms)에 충분합니다.src/main/java/com/gpt/geumpumtabackend/statistics/dto/DayMaxFocusAndFullTimeStatistics.java (1)
4-6: LGTM!일일 통계이므로
Integer타입이 적절합니다. 하루 최대 밀리초(86,400,000ms)가Integer.MAX_VALUE보다 훨씬 작습니다.src/main/java/com/gpt/geumpumtabackend/statistics/service/StatisticsService.java (2)
31-31: LGTM! 통계 책임 분리가 잘 되었습니다.
StudySessionRepository에서StatisticsRepository로의 의존성 변경은 단일 책임 원칙(SRP)에 부합하는 좋은 리팩토링입니다.
140-145: 저장소 메서드 시그니처 확인됨 - 이슈 없음
StatisticsRepository.getMonthlyStatistics의 실제 시그니처는 2개의 파라미터(LocalDateTime monthStart,Long userId)를 받으며, 서비스에서 정확히 2개의 인자(monthStart,targetUserId)를 전달하고 있습니다. 시그니처가 일치하므로 컴파일 오류가 발생하지 않습니다.Likely an incorrect or invalid review comment.
src/main/java/com/gpt/geumpumtabackend/statistics/dto/MonthlyStatistics.java (1)
4-7: LGTM!타입 선택과 주석이 잘 작성되었습니다.
Long은 월간 총 밀리초에 적합하고,Integer는 일일 평균 및 일수 카운트에 충분합니다.src/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java (3)
1-13: LGTM!패키지 구조와 import가 적절합니다.
JpaRepository<StudySession, Long>를 확장하여 통계 전용 리포지토리로 분리한 것은 단일 책임 원칙(SRP)에 부합합니다.
60-84: LGTM!일일 총 공부시간과 최대 집중시간을 계산하는 쿼리가 올바르게 구현되었습니다.
s.end_time > :dayStart조건으로 진행 중인 세션(NULL end_time)이 자연스럽게 제외됩니다.
268-276: 명시적인 NULL 체크로 완료된 세션만 필터링 - 좋습니다!
s.end_time IS NOT NULL조건을 명시적으로 사용하여 진행 중인 세션을 제외한 것이 좋습니다. 다른 쿼리들도 이와 같이 명시적인 조건을 사용하면 의도가 더 명확해집니다.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
src/main/java/com/gpt/geumpumtabackend/statistics/repository/StatisticsRepository.java
Show resolved
Hide resolved
Juhye0k
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
🚀 1. 개요
📝 2. 주요 변경 사항
📸 3. 스크린샷 (API 테스트 결과)
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.